Skip to content

[WLM] introduce rule cardinality check #18663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Jul 10, 2025
Merged

[WLM] introduce rule cardinality check #18663

merged 7 commits into from
Jul 10, 2025
+92 −2

Conversation

kaushalmahi12
Copy link
Contributor

Description

This change will ensure that user can't create unlimited rule documents. We introducing a limit of 10000.

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@kaushalmahi12 kaushalmahi12 requested a review from a team as a code owner July 1, 2025 20:33
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2025

❌ Gradle check result for 38f2b8b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kaushalmahi12 kaushalmahi12 force-pushed the rule_cardinality_check branch from 22cf342 to fcb317f Compare July 1, 2025 21:19
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2025

❌ Gradle check result for fcb317f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Jul 1, 2025
Open
@kaushalmahi12 kaushalmahi12 force-pushed the rule_cardinality_check branch from fcb317f to e8494d6 Compare July 9, 2025 18:26
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for 4a606b2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kaushalmahi12
add cardinality check
27b5bde 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12
add rule cardinality check UT
b952899 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12
add max rule setting
84a02b9 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12
add changelog
4760944 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12
change javadoc format
70a8f3d 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12
address comments
530303d 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12 kaushalmahi12 force-pushed the rule_cardinality_check branch from 4a606b2 to 530303d Compare July 9, 2025 20:54
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for 530303d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@jainankitk
Copy link
Contributor

Failures look related:

[Test Result](https://build.ci.opensearch.org/job/gradle-check/60221/testReport/) (3 failures / +3)

    [org.opensearch.plugin.wlm.WorkloadManagementPluginTests.testCreateComponentsReturnsRefreshMechanism](https://build.ci.opensearch.org/job/gradle-check/60221/testReport/junit/org.opensearch.plugin.wlm/WorkloadManagementPluginTests/testCreateComponentsReturnsRefreshMechanism/)
    [org.opensearch.plugin.wlm.WorkloadManagementPluginTests.testCreateComponentsInitializesRulePersistenceService](https://build.ci.opensearch.org/job/gradle-check/60221/testReport/junit/org.opensearch.plugin.wlm/WorkloadManagementPluginTests/testCreateComponentsInitializesRulePersistenceService/)
    [org.opensearch.plugin.wlm.WorkloadManagementPluginTests.testGetFeatureTypeReturnsWorkloadGroupFeatureType](https://build.ci.opensearch.org/job/gradle-check/60221/testReport/junit/org.opensearch.plugin.wlm/WorkloadManagementPluginTests/testGetFeatureTypeReturnsWorkloadGroupFeatureType/)

jainankitk
jainankitk approved these changes Jul 9, 2025
View reviewed changes
Copy link
Contributor

@jainankitk jainankitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for c33141d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kaushalmahi12
register new setting
1af2c8e 
Signed-off-by: Kaushal Kumar <[email protected]>
@kaushalmahi12 kaushalmahi12 force-pushed the rule_cardinality_check branch from c33141d to 1af2c8e Compare July 9, 2025 23:52
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for 1af2c8e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Jul 10, 2025
Open
@jainankitk
Copy link
Contributor

Seems like flaky test:

[Test Result](https://build.ci.opensearch.org/job/gradle-check/60235/testReport/) (1 failure / +1)

    [org.opensearch.index.shard.IndexShardTests.testRestoreSearchOnlyShardFromStoreOnNewNode](https://build.ci.opensearch.org/job/gradle-check/60235/testReport/junit/org.opensearch.index.shard/IndexShardTests/testRestoreSearchOnlyShardFromStoreOnNewNode/)

@github-actions GitHub Actions
Copy link
Contributor

❕ Gradle check result for 1af2c8e: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov Codecov
Copy link

codecov bot commented Jul 10, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.81%. Comparing base (c2f41cf) to head (1af2c8e).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ule/service/IndexStoredRulePersistenceService.java 80.00% 1 Missing and 1 partial ⚠️
...search/rule/storage/IndexBasedRuleQueryMapper.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #18663      +/-   ##
============================================
+ Coverage     72.78%   72.81%   +0.02%     
- Complexity    68527    68532       +5     
============================================
  Files          5572     5572              
  Lines        314779   314790      +11     
  Branches      45691    45692       +1     
============================================
+ Hits         229115   229216     +101     
+ Misses        67080    66923     -157     
- Partials      18584    18651      +67

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jainankitk jainankitk merged commit 5ac8177 into opensearch-project:main Jul 10, 2025
32 of 34 checks passed
This was referenced Jul 10, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkhatua kkhatua kkhatua left review comments

@jainankitk jainankitk jainankitk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kaushalmahi12 @jainankitk @kkhatua